Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: automatic k8 operator token refreshing #2620

Merged
merged 4 commits into from
Oct 24, 2024

Conversation

DanielHougaard
Copy link
Contributor

Description 📣

This PR introduces automatic token refreshing, which is handled internally by our Go Lang SDK.

Type ✨

  • Bug fix
  • New feature
  • Improvement
  • Breaking change
  • Documentation

@DanielHougaard DanielHougaard self-assigned this Oct 18, 2024
Copy link
Collaborator

@maidul98 maidul98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feature seems to be working as expected but it keeps saying it wasn't able to renew periodically even though in the next iteration, it can. When we know the ttl will exceed the max ttl, we can just login instead of using the expired creds.

CleanShot 2024-10-22 at 23 18 49@2x

Here some other comments:

  • I don't think we need to paint the logs yellow
  • The logs say that it wasn't able to renew but i think it does? The user is never made aware that it has renewed successfully after though
  • I think our approach with logging might be incorrect for the operator. When there are are more than 2 InfisicalSecret CRD, these logs aren't useful anymore because they don't contain any context for which CRD they are for. I would had some context to the SDK logs for now. I'll make a linear item to revamp our operator logs in a future PR.
CleanShot 2024-10-22 at 23 08 41@2x

Copy link
Collaborator

@maidul98 maidul98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: ask updates applied. lgtm

@maidul98 maidul98 merged commit 71081d8 into main Oct 24, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants